Skip to content

31800: validate Incorporation Application share structure fields#4036

Open
loneil wants to merge 10 commits intobcgov:mainfrom
loneil:31800shareValidationsForIa
Open

31800: validate Incorporation Application share structure fields#4036
loneil wants to merge 10 commits intobcgov:mainfrom
loneil:31800shareValidationsForIa

Conversation

@loneil
Copy link
Collaborator

@loneil loneil commented Jan 28, 2026

Issue #: /bcgov/entity#31800

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the lear license (Apache 2.0).

Copilot AI review requested due to automatic review settings January 28, 2026 16:30
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens validation rules for share structures, enforcing consistent naming for share classes and series and adding comprehensive unit tests around these rules.

Changes:

  • Introduced EXCLUDED_WORDS_FOR_CLASS and EXCLUDED_WORDS_FOR_SERIES constants and extended validate_shares / validate_series to enforce a " Shares" suffix and to reject reserved words in class and series names.
  • Ensured series-level validations (max shares, rights/restrictions, duplicate names) now also include suffix and reserved-word checks while maintaining existing behavior.
  • Added extensive unit tests in test_common_validations.py for share-class and series names, as well as for end-to-end validate_share_structure behavior over various valid and invalid share structures.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
legal-api/src/legal_api/services/filings/validations/common_validations.py Adds reserved-word and suffix validations for share class and series names, and exposes exclusion word lists as constants used across filings.
legal-api/tests/unit/services/filings/validations/test_common_validations.py Adds parameterized tests covering class/series name suffix rules, reserved words, whitespace/emptiness, duplication, and full share-structure validation behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@loneil loneil marked this pull request as draft January 29, 2026 05:40
@loneil loneil changed the title validate share structure 31800: validate Incorporation Application share structure fields Jan 29, 2026
@loneil loneil force-pushed the 31800shareValidationsForIa branch 3 times, most recently from a6477dd to 3895c05 Compare February 2, 2026 23:55
)


def has_at_least_one_share_class(filing_json, filing_type) -> Optional[str]: # pylint: disable=too-many-branches
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found this function was unused anywhere in the app (validation was done elsewhere)

Signed-off-by: Lucas <lucasoneil@gmail.com>
Signed-off-by: Lucas <lucasoneil@gmail.com>
Signed-off-by: Lucas <lucasoneil@gmail.com>
Signed-off-by: Lucas <lucasoneil@gmail.com>
Signed-off-by: Lucas <lucasoneil@gmail.com>
Signed-off-by: Lucas <lucasoneil@gmail.com>
Signed-off-by: Lucas <lucasoneil@gmail.com>
@loneil loneil force-pushed the 31800shareValidationsForIa branch from 2762cfb to f1fd9f7 Compare February 4, 2026 06:15
Signed-off-by: Lucas <lucasoneil@gmail.com>
Signed-off-by: Lucas <lucasoneil@gmail.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@loneil loneil marked this pull request as ready for review February 4, 2026 21:34
Signed-off-by: Lucas <lucasoneil@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant